Skip to content

Conversation

@mapiolca
Copy link
Owner

@mapiolca mapiolca commented Oct 8, 2025

No description provided.

@mapiolca mapiolca merged commit c3304d8 into 1.0 Oct 8, 2025
1 check passed
mapiolca added a commit that referenced this pull request Oct 8, 2025
Merge pull request #22 from mapiolca/main
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines +274 to +278
$sql = "SELECT rowid FROM ".MAIN_DB_PREFIX."timesheet_week_line";
$sql .= " WHERE fk_timesheet_week=".(int) $this->id;
// EN: Protect the fetch to ensure lines belong to an allowed entity scope.
// FR: Protège le chargement pour s'assurer que les lignes appartiennent à une entité autorisée.
$sql .= " AND entity IN (".getEntity('timesheetweek').")";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Filtering lines by entity hides pre-existing data

The new fetchLines() clause now restricts rows to entity IN (getEntity('timesheetweek')). Prior to this commit, timesheet lines were inserted without filling the entity column (see previous code), so historical data created in another entity still carries the default value 1. After deployment, fetching a sheet from a shared entity will no longer load any of its existing lines and related actions (submit, totals, etc.) will fail because those rows are filtered out. A data migration or a backward‑compatible filter is needed before this check is applied.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants